Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support CRYSTAL_LIBRARY_RPATH for adding dynamic library lookup paths #13499

Merged

Conversation

HertzDevil
Copy link
Contributor

Resolves #13490, except for the interpreter part.

The CRYSTAL_LIBRARY_RPATH environment variable can be used to pass a list of directories used by built programs to locate their dynamic libraries at run time. $ORIGIN and ${ORIGIN} are expanded into the program's absolute path, and this PR adds a custom implementation for this on Windows, only available if the delay-load helper is present (i.e. -Dno_win32_delay_load is not used). Linux's loader also expands $LIB and $PLATFORM, although this is not guaranteed.

The compiler will automatically add its own RPATH to anything that needs to be compiled and run immediately as a child process. For Windows, this means as long as the compiler can find its own DLLs, crystal run -Dpreview_dll / eval / spec or any macro run inside crystal build -Dpreview_dll will always be able to locate the same DLLs distributed along the compiler itself, even if the compiler is not in %PATH%. In principle, if the compiler itself is dynamically linked with a CRYSTAL_LIBRARY_RPATH containing $ORIGIN, then the DLLs do not even have to share the same directory as the compiler.

For most Unix-like systems, this automatically passes -Wl,-rpath,... to the linker. It seems linkers disagree on whether that option affects RPATH or RUNPATH. macOS is excluded because it handles library lookup differently.

The compiler automatically generates the Crystal::LIBRARY_RPATH constant; when a Crystal source is run, this constant will reflect the contents of both the environment variable and the compiler itself, as described above. The standard library also defines it explicitly in case a compiler without this PR tries to use it.

src/crystal/system/win32/delay_load.cr Show resolved Hide resolved
src/crystal/system/win32/delay_load.cr Show resolved Hide resolved
src/crystal/system/win32/delay_load.cr Outdated Show resolved Hide resolved
src/compiler/crystal/codegen/link.cr Outdated Show resolved Hide resolved
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
I have an optional suggestion, but I'm happy with or without it.

@@ -334,7 +334,7 @@ class Crystal::Command
output_format : String?,
combine_rpath : Bool do
def compile(output_filename = self.output_filename)
compiler.emit_base_filename = original_output_filename
compiler.emit_base_filename = output_filename.rchop(File.extname(output_filename))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent and more concise:

Suggested change
compiler.emit_base_filename = output_filename.rchop(File.extname(output_filename))
compiler.emit_base_filename = Path[output_filename].stem

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from a prior PR. That PR actually changed the behavior of -o so that it also affects --emit; previously those emitted files would always be in the current working directory and ignore the name given to -o. Thus output_filename could now contain directory separators, and calling Path#stem would remove them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the name filename is misleading then if it's actually a path.
But okay, don't need to fix that here.

@straight-shoota straight-shoota added this to the 1.9.0 milestone May 28, 2023
@straight-shoota straight-shoota merged commit 76c24b0 into crystal-lang:master Jun 5, 2023
46 checks passed
@HertzDevil HertzDevil deleted the feature/crystal-library-rpath branch June 5, 2023 14:39
Blacksmoke16 pushed a commit to Blacksmoke16/crystal that referenced this pull request Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Overriding the default dynamic library lookup path
2 participants